AudioStream: remove unused youtube-specific fields#1150
AudioStream: remove unused youtube-specific fields#1150Profpatsch wants to merge 1 commit intoTeamNewPipe:devfrom
Conversation
These fields were added in TeamNewPipe#810 in preparation for a DASH support that never materialized … well it did materialize, but did not use any of these variables. Since they are youtube-specific, it does not make much sense to export them in the generic API, lest people start depending on them as if they belonged to all backends. Well, 4 variables are actually already depended on in places, they will have to be checked separately. But this is the low-hanging fruit.
Profpatsch
force-pushed
the
AudioStream-remove-unused-fields
branch
from
a719b7d to
8448f0a
Compare
TobiGr
added
youtube
TobiGr
left a comment
TobiGr
left a comment
There was a problem hiding this comment.
Thanks for the PR!
YoutubeDashManifestCreatorsTest.testProgressiveStreams() fails locally.
| // Fields for DASH | ||
| private int itag = ITAG_NOT_AVAILABLE_OR_NOT_APPLICABLE; | ||
| private int bitrate; | ||
| private int initStart; | ||
| private int initEnd; | ||
| private int indexStart; | ||
| private int indexEnd; | ||
| private String quality; | ||
| private String codec; |
There was a problem hiding this comment.
Hi, I was the one who added these fields originally, they are used in Piped's backend for providing sidX boxes for streaming with DASH.
Here's how an audio representation is created by a client: https://github.com/TeamPiped/Piped/blob/8ee2a1c20737ce9db84186e00849b405d6c097af/src/utils/DashUtils.js#L99-L117
There was a problem hiding this comment.
Then maybe a comment should be added that these were added for Piped? Similar confusion can take place whenever someone compares NP and NPE code in the future.
|
Closing to not brake Piped |
4 participants
These fields were added in
#810 in preparation for a DASH support that never materialized … well it did materialize, but did not use any of these variables.
Since they are youtube-specific, it does not make much sense to export them in the generic API, lest people start depending on them as if they belonged to all backends.
Well, 4 variables are actually already depended on in places, they will have to be checked separately. But this is the low-hanging fruit.